add task solution react_decompose#4478
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because a core requirement of the task is not yet implemented, which makes the solution incomplete. The description explicitly states that the navigation must be moved into its own Navigation component (with its own folder and CSS), but the <nav className="navigation"> block and its links are still defined directly in src/components/Header/Header.jsx and styled in src/components/Header/Header.css instead of being extracted. This also means [CHECKLIST ITEM #2] (separate folder per component, including markup and styles) is not fully satisfied, since there is no Navigation component folder/file in src/components/Navigation/.
To fix this, you should:
- Create a
src/components/Navigation/Navigation.jsxandNavigation.css. - Move the
<nav>markup and related navigation styles (.navigation,.navigation__link, etc.) fromHeader.jsx/Header.cssinto those new files. - Import and render
<Navigation />insideHeader.jsxand ensureAppstill composesWelcome,Header, andArticlecorrectly.
Everything else (splitting App into Welcome, Header, and Article with default exports and per-component CSS) looks good, so once you extract the navigation into its own component and folder, your solution should meet the task requirements.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| return ( | ||
| <article className="article"> |
There was a problem hiding this comment.
The task description requires splitting App.js into components based on CSS blocks, including moving navigation into its own component. Here the App component only imports Welcome, Header, and Article; a separate Navigation component is missing, which means the navigation block is still coupled to Header and the requirement is not fully satisfied.
| @@ -3,3 +3,71 @@ | |||
| // Create an Article function returning the HTML of article block | |||
|
|
|||
| // Add a default export statement for Article component to use it in the other files | |||
There was a problem hiding this comment.
Checklist item #2 says: "create separate folder per component, where you could put all files(styles, components, and so on)". Since there is no separate Navigation component folder/file, the navigation block’s markup and styles are not split into their own component folder as implied by the description and checklist.
| .header { | ||
| position: sticky; | ||
| display: flex; | ||
| } | ||
|
|
||
| .header__title { | ||
| font-weight: 600; | ||
| } | ||
|
|
||
| .navigation__link { | ||
| opacity: 0.7; | ||
| } | ||
|
|
||
| @media (min-width: 768px) { | ||
| .header { | ||
| display: flex; | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
The task description requires: "Move the navigation into a Navigation component; you may create a folder for the Navigation component and move navigation there." Right now, the navigation markup is still inside Header, so you'll need to extract it into a separate Navigation component and import/use it here.
| <nav className="navigation"> | ||
| <a className="navigation__link" href="#about"> | ||
| About | ||
| </a> | ||
| <a className="navigation__link" href="#services"> | ||
| Services | ||
| </a> | ||
| <a className="navigation__link" href="#contact"> | ||
| Contact | ||
| </a> | ||
| </nav> |
There was a problem hiding this comment.
This violates the description requirement to "Move the navigation into a Navigation component". The <nav className="navigation"> block and its links should be extracted into a separate Navigation component and imported/used in Header instead of being defined inline here.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution because all core requirements visible in the code are correctly implemented. App is properly decomposed into Welcome, Header, and Article, and you’ve also extracted Navigation into its own component and folder with its own CSS, which addresses the earlier critical issue and satisfies [CHECKLIST ITEM #2]. All React components use export default as required by the tests, and there’s no overuse of empty lines between components, so [CHECKLIST ITEM #1] is also respected. From the code shown, there are no functional or structural blockers that would prevent the task from being completed.
The only remaining points are process-related and not visible in the repository snapshot: make sure all new files (especially Navigation.jsx and Navigation.css) are added and committed to git for [CHECKLIST ITEM #3], run npm test locally to confirm everything passes, and ensure your PR DEMO link uses your actual GitHub username. Overall, this is a clean, well-structured implementation that matches the task description—well done.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
No description provided.